Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize counter initialization by reducing the number of bulk counter poll calls and communication between swss/sairedis #1527

Merged
merged 32 commits into from
Feb 19, 2025

Conversation

stephenxs
Copy link
Contributor

@stephenxs stephenxs commented Feb 9, 2025

Optimize counter initialization by reducing the number of bulk counter poll calls and communication between swss(orchagent)/sairedis(syncd) during initialization.

Originally, orchagent notifies syncd to initialize the counter using an extended sairedis call SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER for each SAI object with the object ID as the key, which means the number of the extended sairedis calls is identical as the number of objects. It takes time to finish all the extended sairedis calls.

Now, for counter groups that have many objects (e.g., port, PG, queues, etc), orchagent notifies syncd to initialize the counter using a single extend sairedis call with many objects' ID as the key (format: <key1>,<key2>,...<keyn>). So, it takes much less time to initialize the counters because fewer extend sairedis calls are required.

Details:

  • In sairedis, the bulk counter is supported for all counter groups except Buffer Pool Counter and DASH ENI counter.

  • In swss, bulk counter for the following counter groups

    • priority group watermark
    • priority group drop
    • queue watermark
    • queue stat
    • PFC watchdog

HLD sonic-net/SONiC#1862

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -2618,6 +2819,171 @@
notifyPoll();
}

void FlexCounter::bulkAddCounter(

Check notice

Code scanning / CodeQL

Irregular enum initialization Note

In an enumerator list, the = construct should not be used to explicitly initialize members other than the first, unless all items are explicitly initialized.
@stephenxs
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs stephenxs requested review from kcudnik and keboliu February 10, 2025 08:53
@stephenxs stephenxs marked this pull request as ready for review February 13, 2025 11:59
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Contributor Author

the failure is not relevant to the commits

[ RUN      ] ClientServerSai.OUTBOUND_CA_TO_PA_ENTRY
Bad file descriptor (src/signaler.cpp:345)
/bin/bash: line 6: 25244 Aborted                 (core dumped) ${dir}$tst
FAIL: tests

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Stephen Sun <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

kcudnik
kcudnik previously approved these changes Feb 16, 2025
All counters are created in bulk mode.
We should translate bulkAddObject to iteration of single call (addObject) for counter groups that do not support bulk.

Signed-off-by: Stephen Sun <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Contributor Author

Hi @kcudnik would you please review it again and merge it if no comments? fix an issue since last approval and also add ut to cover it.
thanks

@stephenxs stephenxs requested a review from kcudnik February 18, 2025 06:05
@kcudnik kcudnik merged commit 24aed42 into sonic-net:master Feb 19, 2025
15 checks passed
@stephenxs stephenxs deleted the bulk-counter-init branch February 19, 2025 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants